Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Handle platforms without compaction support #275

Merged
merged 1 commit into from
Jun 29, 2022
Merged

Handle platforms without compaction support #275

merged 1 commit into from
Jun 29, 2022

Conversation

casperisfine
Copy link

@casperisfine casperisfine commented Apr 7, 2022 •
edited by byroot
Loading

Fix: #265
Ref: https://bugs.ruby-lang.org/issues/18779

That's a test only concern, so no big deal.

cc @pvalena

@pvalena
Copy link

pvalena commented Apr 8, 2022

Thanks!

@byroot byroot requested a review from chrisseaton June 29, 2022 14:40
chrisseaton
chrisseaton approved these changes Jun 29, 2022
View reviewed changes
Copy link
Contributor

@chrisseaton chrisseaton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think GC.respond_to?(:verify_compaction_references) is the better pattern, but not a big deal.

@byroot
Copy link
Member

byroot commented Jun 29, 2022

It definitely is the better pattern, unfortunately some platforms have the method defined but it raises NotImplementedError if called. That was fixed on master, but we still need to handle older rubies.

@byroot byroot merged commit 23339e0 into msgpack:master Jun 29, 2022
@matoro matoro mentioned this pull request Nov 21, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chrisseaton chrisseaton chrisseaton approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Compaction isn't available on this platform
4 participants
@casperisfine @pvalena @byroot @chrisseaton